Skip to content

[sandbox audit] Recycle sandbox uids, but only when teardown is verified - #23

Draft
Wauplin wants to merge 1 commit into
security/opaque-process-idsfrom
security/recycle-sandbox-uids
Draft

[sandbox audit] Recycle sandbox uids, but only when teardown is verified#23
Wauplin wants to merge 1 commit into
security/opaque-process-idsfrom
security/recycle-sandbox-uids

Conversation

@Wauplin

@Wauplin Wauplin commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

[sandbox audit] — PR 9 of 13 in this repo's stack; merge in order.
Previous: #22 · Next: #25
Review only the commits this PR adds on top of its base; bases collapse to main as the stack lands.

Why

Uids were allocated monotonically from 20000 and never reused. A host that created ~45,000
sandboxes over its 24h lifetime could no longer create one even while empty.

Worse than the exhaustion itself: it surfaced as a 500, which the client reads as a hard
error rather than "this host is full" — so the sandbox wasn't re-placed onto another host, the
create() just failed.

Approach

Reuse is the fix, and a careless free list would be worse than the problem: hand back a uid
whose processes are still alive and the next sandbox inherits them, with the ability to signal
and read them. So:

  • a uid is freed only when teardown is known to have converged — which the previous PR's
    kill_uid return value makes knowable;
  • anything doubtful is quarantined for the process's lifetime: surviving processes, or a
    home directory that couldn't be removed (files owned by that uid may remain);
  • and it is re-checked at hand-out, because a decision made a second ago is not a
    guarantee about now.

Losing one uid out of 45,000 is the cheap side of that trade.

Exhaustion now reports CreateError::Full, so the client packs elsewhere.

Also: at startup, sample the uids the running image already uses in our range — from
/etc/passwd and from any process already running under one — and reserve them. UID_BASE = 20000 was chosen on the assumption that images don't use it, but nothing checked, and a
collision would put two "isolated" sandboxes under a single uid, defeating the entire DAC half
of the isolation model.

Validation

  • 48 unit tests, including: reuse ordering; quarantine honoured from either direction;
    image-reserved uids skipped; exhaustion stable rather than intermittent; UID_BASE/UID_MAX
    staying inside the container's 0..65535 uid map; and that a "free" uid which still owns
    processes is quarantined at hand-out rather than reused (using our own uid, which certainly
    has one).

  • Live, root container:

    12 sequential create/delete cycles -> uids handed out: 20000 (×12), distinct: 1
    4 concurrently live sandboxes      -> distinct uids: 4
    

    Both directions matter: recycling that also collided between live sandboxes would be a far
    worse bug than the one being fixed.

Depends on

The honest kill_uid return value from #21 — without it there is no way to know a uid is safe
to reuse, and this PR would be the dangerous kind of fix.

Uids were allocated monotonically from 20000 and never reused, so a host
that created ~45,000 sandboxes over its 24h lifetime could no longer create
one even while empty. Worse, exhaustion surfaced as a 500, which the client
reads as a hard error rather than "this host is full" -- so the sandbox was
not re-placed onto another host, it just failed.

Reuse is the fix, and a careless free list would be worse than the problem:
hand back a uid whose processes are still alive and the next sandbox
inherits them, with the ability to signal and read them. So the pool only
frees a uid when teardown is *known* to have converged (which the previous
commit's `kill_uid` return value makes knowable), quarantines it for the
process's lifetime on any doubt -- surviving processes, or a home directory
that could not be removed -- and re-checks at hand-out, because a decision
made a second ago is not a guarantee about now. Losing one uid out of 45,000
is the cheap side of that trade.

Exhaustion now reports `CreateError::Full`, so the client packs elsewhere.

Also samples, at startup, the uids the running image already uses in our
range -- from `/etc/passwd` and from any process already running under one --
and reserves them. `UID_BASE = 20000` was chosen on the assumption that
images do not use it, but nothing checked, and a collision would put two
"isolated" sandboxes under a single uid, defeating the whole DAC half of the
isolation model.

Validation:
- 48 unit tests, including reuse ordering, quarantine being honoured from
  either direction, image-reserved uids being skipped, exhaustion being
  stable rather than intermittent, `UID_BASE`/`UID_MAX` staying inside the
  container's uid map, and that a "free" uid which still owns processes is
  quarantined at hand-out instead of reused (using our own uid, which
  certainly has one).
- Live in a root container: 12 sequential create/delete cycles now hand out
  a single uid instead of 12, while 4 concurrently live sandboxes still get
  4 distinct ones.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Wauplin Wauplin changed the title Recycle sandbox uids, but only when teardown is verified [sandbox audit] Recycle sandbox uids, but only when teardown is verified Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant